Skip to content

Conversation

@fneddy
Copy link
Contributor

@fneddy fneddy commented Nov 18, 2025

on s390x 128bit types have a smaller alignment then on x861. This leads to smaller structs (24 instead of 32 bytes) and therefore the write_unaligned will write outside of the structs boundary.

To fix the test, change the trailing u32 into a u8. This will generate 7 trailing padding bytes on s390x and 15 on x86_64. Also change the start of the garbage data so it will span over x,y and padding:

s390x:   XXXXXXXXXXXXXXXXY-------
x86_64:  XXXXXXXXXXXXXXXXY---------------
                      GGGGGGGG

Footnotes

  1. s390x ELF ABI Table 1.1, Page 12 https://github.com/IBM/s390x-abi

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 18, 2025

r? @madsmtm

rustbot has assigned @madsmtm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@fneddy
Copy link
Contributor Author

fneddy commented Nov 18, 2025

tagging @RalfJung who introduced the test in #148259

@RalfJung
Copy link
Member

Given that what we are testing here is entirely target-independent code (the handling of provenance in the interpreter), I think I'd prefer just skipping the test on s390x over the risk of making the test not test the right thing any more.

@RalfJung
Copy link
Member

This leads to smaller structs (24 instead of 32 bytes) and therefore the write_unaligned will write outside of the structs boundary.

If you add align(16) to the struct Pair, does that suffice? That should make it 32 bytes large, right?

@RalfJung
Copy link
Member

Actually never mind that, repr(align) would make this not be a ScalarPair any more.

@fneddy
Copy link
Contributor Author

fneddy commented Nov 18, 2025

ok for me. I'll revert the test and disable the test on s390x.

fneddy added a commit to fneddy/rust that referenced this pull request Nov 18, 2025
on s390x 128bit types have a smaller alignment then on x86[^1].
This leads to smaller structs and therefore the write_unaligned will
write outside of the structs boundary.

For now disable the tests on s390x. [^2]

[^1]: s390x ELF ABI Table 1.1, Page 12 https://github.com/IBM/s390x-abi
[^2]: rust-lang#149056 (comment)
@fneddy fneddy force-pushed the fix_s390_test_padding_overwrite branch from eae45d8 to e58572b Compare November 18, 2025 14:02
@RalfJung
Copy link
Member

This looks great, thanks! Please squash the commits, then we can land this. Please use the --keep-base flag when squashing so that the force-push diff is easier to review; do not rebase on the latest master branch. Then write @rustbot ready after you force-pushed the squashed PR.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 19, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 19, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

on s390x 128bit types have a smaller alignment then on x86[^1].
This leads to smaller structs and therefore the write_unaligned will
write outside of the structs boundary.

For now disable the tests on s390x. [^2]

[^1]: s390x ELF ABI Table 1.1, Page 12 https://github.com/IBM/s390x-abi
[^2]: rust-lang#149056 (comment)

Co-authored-by: Ralf Jung <post@ralfj.de>
@fneddy fneddy force-pushed the fix_s390_test_padding_overwrite branch from 425b450 to 10172d1 Compare November 19, 2025 08:49
@fneddy
Copy link
Contributor Author

fneddy commented Nov 19, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 19, 2025
@RalfJung
Copy link
Member

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 19, 2025

📌 Commit 10172d1 has been approved by RalfJung

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 19, 2025
@madsmtm madsmtm assigned RalfJung and unassigned madsmtm Nov 19, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 19, 2025
…rite, r=RalfJung

fix the fragment_in_dst_padding_gets_overwritten test on s390x

on s390x 128bit types have a smaller alignment then on x86[^1]. This leads to smaller structs (24 instead of 32 bytes) and therefore the write_unaligned will write outside of the structs boundary.

To fix the test, change the trailing u32 into a u8. This will generate 7 trailing padding bytes on s390x and 15 on x86_64. Also change the start of the garbage data so it will span over x,y and padding:

```
s390x:   XXXXXXXXXXXXXXXXY-------
x86_64:  XXXXXXXXXXXXXXXXY---------------
                      GGGGGGGG

```

[^1]: s390x ELF ABI Table 1.1, Page 12 https://github.com/IBM/s390x-abi
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests

Successful merges:

 - #146341 (minimal dirfd implementation (1/4))
 - #146925 (Add doc for va_list APIs)
 - #147035 (alloc: fix `Debug` implementation of `ExtractIf`)
 - #147173 (Add support for hexagon-unknown-qurt target)
 - #149041 (ignore unsized types in mips64 and sparc64 callconvs)
 - #149056 (fix the fragment_in_dst_padding_gets_overwritten test on s390x)
 - #149095 (rustc-dev-guide subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 19, 2025
…rite, r=RalfJung

fix the fragment_in_dst_padding_gets_overwritten test on s390x

on s390x 128bit types have a smaller alignment then on x86[^1]. This leads to smaller structs (24 instead of 32 bytes) and therefore the write_unaligned will write outside of the structs boundary.

To fix the test, change the trailing u32 into a u8. This will generate 7 trailing padding bytes on s390x and 15 on x86_64. Also change the start of the garbage data so it will span over x,y and padding:

```
s390x:   XXXXXXXXXXXXXXXXY-------
x86_64:  XXXXXXXXXXXXXXXXY---------------
                      GGGGGGGG

```

[^1]: s390x ELF ABI Table 1.1, Page 12 https://github.com/IBM/s390x-abi
bors added a commit that referenced this pull request Nov 20, 2025
Rollup of 6 pull requests

Successful merges:

 - #146925 (Add doc for va_list APIs)
 - #147035 (alloc: fix `Debug` implementation of `ExtractIf`)
 - #147173 (Add support for hexagon-unknown-qurt target)
 - #149041 (ignore unsized types in mips64 and sparc64 callconvs)
 - #149056 (fix the fragment_in_dst_padding_gets_overwritten test on s390x)
 - #149095 (rustc-dev-guide subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 20, 2025
…rite, r=RalfJung

fix the fragment_in_dst_padding_gets_overwritten test on s390x

on s390x 128bit types have a smaller alignment then on x86[^1]. This leads to smaller structs (24 instead of 32 bytes) and therefore the write_unaligned will write outside of the structs boundary.

To fix the test, change the trailing u32 into a u8. This will generate 7 trailing padding bytes on s390x and 15 on x86_64. Also change the start of the garbage data so it will span over x,y and padding:

```
s390x:   XXXXXXXXXXXXXXXXY-------
x86_64:  XXXXXXXXXXXXXXXXY---------------
                      GGGGGGGG

```

[^1]: s390x ELF ABI Table 1.1, Page 12 https://github.com/IBM/s390x-abi
bors added a commit that referenced this pull request Nov 20, 2025
Rollup of 9 pull requests

Successful merges:

 - #146925 (Add doc for va_list APIs)
 - #147035 (alloc: fix `Debug` implementation of `ExtractIf`)
 - #147173 (Add support for hexagon-unknown-qurt target)
 - #148261 (rustc_public: Make Id types !Send / !Sync)
 - #149041 (ignore unsized types in mips64 and sparc64 callconvs)
 - #149043 ( rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution)
 - #149056 (fix the fragment_in_dst_padding_gets_overwritten test on s390x)
 - #149095 (rustc-dev-guide subtree update)
 - #149108 ([AIX][ppc64le-linux-gnu] Add Amy Kwan to target maintainers)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 20, 2025
…rite, r=RalfJung

fix the fragment_in_dst_padding_gets_overwritten test on s390x

on s390x 128bit types have a smaller alignment then on x86[^1]. This leads to smaller structs (24 instead of 32 bytes) and therefore the write_unaligned will write outside of the structs boundary.

To fix the test, change the trailing u32 into a u8. This will generate 7 trailing padding bytes on s390x and 15 on x86_64. Also change the start of the garbage data so it will span over x,y and padding:

```
s390x:   XXXXXXXXXXXXXXXXY-------
x86_64:  XXXXXXXXXXXXXXXXY---------------
                      GGGGGGGG

```

[^1]: s390x ELF ABI Table 1.1, Page 12 https://github.com/IBM/s390x-abi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants